Skip to content

handlers: guard latency metrics when timestamps are absent#1963

Closed
liu-cong with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-end-of-stream-latency-metrics
Closed

handlers: guard latency metrics when timestamps are absent#1963
liu-cong with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-end-of-stream-latency-metrics

Conversation

Copilot AI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

HandleResponseBody unconditionally called all latency metric helpers at end-of-stream. Those helpers enforce timestamp ordering and log errors when timestamps are zero — causing deterministic CI failures in tests that exercise parsing or response-size accumulation without a full request lifecycle (no timing fields set).

Changes

  • pkg/epp/handlers/response.go: Wrap each latency metric call in the presence check it requires:
    • RecordNormalizedTimePerOutputToken / RecordRequestLatencies: only when RequestReceivedTimestamp and ResponseCompleteTimestamp are non-zero
    • RecordRequestTTFT: only when RequestReceivedTimestamp and FirstTokenTimestamp are non-zero
    • RecordRequestTPOT: only when all three timestamps are non-zero
    • RecordResponseSizes: always recorded (no timestamps required)
if endOfStream {
    if !reqCtx.RequestReceivedTimestamp.IsZero() && !reqCtx.ResponseCompleteTimestamp.IsZero() {
        metrics.RecordNormalizedTimePerOutputToken(...)
        metrics.RecordRequestLatencies(...)
    }
    metrics.RecordResponseSizes(...)
    if !reqCtx.RequestReceivedTimestamp.IsZero() && !reqCtx.FirstTokenTimestamp.IsZero() {
        metrics.RecordRequestTTFT(...)
    }
    if !reqCtx.RequestReceivedTimestamp.IsZero() && !reqCtx.FirstTokenTimestamp.IsZero() && !reqCtx.ResponseCompleteTimestamp.IsZero() {
        metrics.RecordRequestTPOT(...)
    }
}
  • pkg/epp/handlers/response_test.go: Add TestHandleResponseBodyZeroTimestamps — verifies that end-of-stream with zero timestamps emits no latency metrics and still records response size.

Production behaviour is unchanged: in a real request all timestamps are set before HandleResponseBody is called, so the guards are never triggered.

When HandleResponseBody reaches end-of-stream it previously called all
latency metric helpers unconditionally, including RecordNormalizedTimePerOutputToken,
RecordRequestLatencies, RecordRequestTTFT, and RecordRequestTPOT.  Those
helpers reject zero/unordered timestamps and log errors, causing
deterministic failures in tests that exercise response parsing or response
size accumulation without initialising timing fields.

Guard each metric call behind the timestamp checks it requires:
- RecordNormalizedTimePerOutputToken and RecordRequestLatencies: only when
  RequestReceivedTimestamp and ResponseCompleteTimestamp are non-zero.
- RecordRequestTTFT: only when RequestReceivedTimestamp and
  FirstTokenTimestamp are non-zero.
- RecordRequestTPOT: only when all three timestamps are non-zero.
- RecordResponseSizes is always recorded at end-of-stream.

Add TestHandleResponseBodyZeroTimestamps to confirm that end-of-stream
handling with zero timestamps records no latency metrics and still records
the response size.

Signed-off-by: GitHub Copilot <copilot@github.com>
@github-actions github-actions Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 10, 2026
Copilot AI changed the title [WIP] Fix end-of-stream latency metrics for requests handlers: guard latency metrics when timestamps are absent Jul 10, 2026
Copilot AI requested a review from liu-cong July 10, 2026 18:12
@ahg-g ahg-g closed this Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants